CASSSIDECAR-411: RangeManager should be singleton in CDCModule#323
CASSSIDECAR-411: RangeManager should be singleton in CDCModule#323jyothsnakonisa merged 6 commits intoapache:trunkfrom
Conversation
yifan-c
left a comment
yifan-c
left a comment
There was a problem hiding this comment.
Please fixed the test failures
jmckenzie-dev
left a comment
jmckenzie-dev
left a comment
There was a problem hiding this comment.
nit on casing and question re: scoping; all things that can be resolved by author during commit, aren't a big deal, or we could follow up on a subsequent commit so we can get this change in.
CHANGES.txt
Outdated
| @@ -1,5 +1,6 @@ | |||
| 0.3.0 | |||
| ----- | |||
| * Rangemanager should be singleton in CDCModule (CASSSIDECAR-411) | |||
There was a problem hiding this comment.
Super nit: RangeManager to match casing on class name. 😅
| } | ||
|
|
||
| protected DnsResolver mockDnsResolver() | ||
| public static DnsResolver mockDnsResolver() |
There was a problem hiding this comment.
Why the increase in scope from private to public? I don't see any new consumers in the PR...
There was a problem hiding this comment.
I am reusing these helper methods in TestModule to fix failing tests.
| } | ||
|
|
||
| private Metadata getMetadata() | ||
| public static Metadata getMetadata() |
There was a problem hiding this comment.
Same as above - purpose of scope promotion? If intentional we should either annotated @VisibleForTesting or javadoc it w/@link so subsequent refactors pull this in / break / make noise in an obvious way.
jyothsnakonisa
changed the title
4 participants
No description provided.